Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gles): support gles backend on openharmony #7085

Merged
merged 4 commits into from
Feb 13, 2025

Conversation

richerfu
Copy link
Contributor

@richerfu richerfu commented Feb 8, 2025

Connections
Link to the issues addressed by this PR, or dependent PRs in other repositories

Description
Support gles render for OpenHarmony.

Testing
There isn't a public openharmony emulator to run test. So we can't run test.

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@richerfu richerfu requested a review from a team as a code owner February 8, 2025 03:41
@richerfu richerfu changed the title fix(gles): fix gles backend crash on openharmony feat(gles): support gles backend on openharmony Feb 8, 2025
@cwfitzgerald
Copy link
Member

Did you at least test this locally? I'm hesitant to include support if it's been completely untested.

@richerfu
Copy link
Contributor Author

richerfu commented Feb 11, 2025

Did you at least test this locally? I'm hesitant to include support if it's been completely untested.

Sure, i test it with wgpu-template and change to the following code:

// graphics.rs
pub async fn create_graphics() {
    // old
    // let instance = Instance::default();
    
    let wids = wgpu::InstanceDescriptor {   
        backends: wgpu::Backends::GL, 
        flags: Default::default(),
        backend_options: Default::default()
    };
    let instance = Instance::new(&wids);

You can see the source code with wgpu-demo-with-winit

And vulkan mode need to update the latest ash version see detail ash-rs/ash#983

@Wumpf
Copy link
Member

Wumpf commented Feb 11, 2025

how hard would it be to add a (build-only) ci test for this target? If that's just another rust toolchain to include that should be definitely done, otherwise it's just a question of time when this breaks again

@cwfitzgerald
Copy link
Member

cwfitzgerald commented Feb 11, 2025

Even if it's tier3, we already have a tier3 path in CI, so shouldn't be that bad

@richerfu
Copy link
Contributor Author

richerfu commented Feb 12, 2025

how hard would it be to add a (build-only) ci test for this target? If that's just another rust toolchain to include that should be definitely done, otherwise it's just a question of time when this breaks again

Right, i will add it. But when i use this patch to test examples with wgpu-in-app, shadow and water crashed. I think i need to fix them before merging this PR.

@cwfitzgerald
Copy link
Member

Don't feel pressured to fix those first. We can land support one step at a time.

@richerfu
Copy link
Contributor Author

68afeb5
I've already add it, but i'm not certain if it aligns with your expectations.Hope your fallback and i'll fix it try my best :). @Wumpf

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@cwfitzgerald cwfitzgerald enabled auto-merge (squash) February 13, 2025 02:54
@cwfitzgerald cwfitzgerald merged commit ff90773 into gfx-rs:trunk Feb 13, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants